Yahoo Ads: Migrate to OpenRTB 2.6#4521
Conversation
Bump the x-openrtb-version header from 2.5 to 2.6 and stop down-converting the auction request inside the bidder. The bidder now passes the 2.6 request through, relying on the PBS-Java up-converter to normalize gdpr, us_privacy, consent, eids, schain and rwdd to their 2.6 top-level slots. For the three privacy/regulatory fields that neither converter handles (gpp, gpp_sid, coppa), add ext->top promotion in modifyRegs: read the 2.6 top-level value, falling back to the legacy 2.5 regs.ext property, and strip the promoted keys from ext. Typed ext-only fields (gpc, dsa) and any other ext properties are preserved. No cattax default is synthesized. Remove the now-unused BidRequestOrtbVersionConversionManager dependency from the bidder and its Spring configuration. Update unit tests for the 2.6 wire shape and add coverage for the ext->top promotion, mixed publisher shapes, wrong-type ext guards, and the no-ext short-circuit. Update the integration fixture to the 2.6 wire shape.
Make stripPromotedFromExt remove a regs.ext key only when its value was actually promoted to top-level (resolved value non-null). A malformed, non-promotable value (e.g. a non-textual gpp) is now left untouched in regs.ext instead of being dropped when a sibling field triggers the rebuild. Add a test covering the mixed case (valid coppa promoted, malformed gpp kept in ext) and trim redundant comments.
resolveGppSid now promotes regs.ext.gpp_sid only when every element is an integer. If any element is non-integral the whole array is left in regs.ext untouched rather than promoting a filtered subset, matching the gpp and coppa handling so no value is silently dropped. Adds a test for the mixed-array case.
Restructure the regs handling so a single per-field signal decides both the promotion and the ext strip. A field is promoted only when it is absent at top-level and present and well-formed in ext; a key is removed from regs.ext if and only if that field was promoted. This removes the previous dependency on whether a sibling field triggered the rebuild (e.g. a value present at both top-level and in ext is now handled the same regardless of siblings). Rename for clarity: modifyRegs -> promoteRegsExtToTopLevel, resolveX -> xToPromote (return the value to lift or null), stripPromotedFromExt -> removePromotedKeys. Add a test for the same field present at both top-level and ext.
|
Hi @osulzhenko , @Fuska1 , @bukrain and @gmiedlar-ox any update on this PR? Thank you. |
Hello. The team is actively working on preparing the 4.0 release. As this requires extensive testing, the codebase is temporarily frozen to not create additional issues that will further delay the 4.0 release. Be assured that the review for this PR is in the queue. Thanks for understanding. |
Thanks @Net-burst! Any rough timeline for the 4.0 release? |
There was a problem hiding this comment.
I'm not sure you need all the promote* methods, since we already do that for you (see BidRequestOrtb25To26Converter).
Also, is it even possible for gpp, gppSid, and coppa to be in regs.ext? We don't even read them from there in the privacy services.
There was a problem hiding this comment.
Thanks for the review @CTMBNara.
I looked into BidRequestOrtb25To26Converter on this. It promotes gdpr and us_privacy up from regs.ext, but it doesn't touch gpp, gpp_sid, or coppa, so those three don't get promoted for us anywhere in core. Let me know if I've missed something there.
Main reason I kept the promotion was it's a no-op for normal requests (values are already top-level, so it skips), but if any arrive under regs.ext they'd otherwise stay nested and get dropped on the way out. Since these are GPP/COPPA signals, I'd rather not risk silently losing one during the cutover. It only kicks in when the top-level field is empty, and leaves everything else in regs.ext untouched.
Resolve conflict in YahooAdsConfiguration by combining upstream's removal of the yahooAds usersyncer with this branch's removal of the OpenRTB version conversion manager, leaving the two-arg YahooAdsBidder(endpoint, mapper).
scr-oath
left a comment
scr-oath
left a comment
There was a problem hiding this comment.
OpenRTB 2.6 migration — review
Headline first: the core design here is correct and necessary, and I want to explicitly back you up on the earlier thread. I independently traced BidRequestOrtb25To26Converter — it promotes only gdpr/us_privacy out of regs.ext and leaves gpp, gpp_sid, and coppa buried as flexible ext properties. So the promote* logic is not redundant with core. The helpers are pure/immutable and well-isolated, and the unit suite is genuinely thorough. 👏
Requesting changes on a small, targeted set:
Should fix before merge
- Silent int-narrowing of
gpp_sid/coppa(see inline).isIntegralNumber()acceptslong/BigInteger, andasInt()then truncates silently — an oversized element passes the "malformed?" guard and ships as a wrong (possibly negative) section ID instead of being left alone.canConvertToInt()(already used inRubiconBidder) closes it in one line. Low probability, but it's privacy data and it defeats the leave-it-untouched guarantee the method advertises. - The integration test doesn't exercise the new promotion path (see inline on the fixture). The incoming IT request already carries
gpp/gpp_sidtop-level, so only core'sgdprpromotion runs and the bidder short-circuits onext == null. Unit coverage is excellent, but the IT gives false confidence for this PR's stated purpose. Concrete fixture edit suggested inline.
Worth a decision (not hard blockers)
3. Stale ext copy left on the wire when the top-level field is already set (see inline) — can put two differing values for one consent signal on the request. It's deliberate + tested, but diverges from core's stricter "once superseded, always clear" (resolveRegsExt). I'd match that pattern.
4. Reachability (the crux of @CTMBNara's second question): AuctionGppService reads gpp/gpp_sid top-level only (no regs.ext fallback), so PBS's own privacy enforcement never sees an ext-nested GPP string. Could you note the concrete upstream request shape that actually delivers gpp/gpp_sid/coppa under regs.ext after core conversion? That would nail down whether the promotion case is reachable in practice.
Follow-ups (separate issues, not this PR)
- Consider promoting
gpp/gpp_sid/coppaonce inBidRequestOrtb25To26Converterso everyortb-version: "2.6"bidder benefits, rather than per-bidder. AuctionGppServicelacking aregs.extfallback for GPP looks like a core-level gap worth its own issue.
Also: the PR currently shows as CONFLICTING — it'll need a rebase on master.
External confirmation (likely yours/Yahoo's to own): dropping the full down-conversion now also sends user.consent/user.eids/source.schain/imp.rwdd top-level. The e2e HTTP 200s are reassuring, but a 200 doesn't prove the SSP honors the TCF string at its new location — worth a signal-level sign-off given the yaml already declared 2.6.
Thanks for the excellent write-up — the field-mapping table and wire-shape matrix made this much easier to review.
| } | ||
| final List<Integer> sids = new ArrayList<>(node.size()); | ||
| for (final JsonNode elem : node) { | ||
| if (!elem.isIntegralNumber()) { |
There was a problem hiding this comment.
issue (non-blocking): isIntegralNumber() is true for long/BigInteger nodes, and asInt() on those silently truncates (3000000000 → -1294967296, 2147483648 → -2147483648). So an out-of-int-range gpp_sid element passes this "malformed?" guard and then gets shipped to the SSP as a corrupted section ID — which defeats the "leave it in ext untouched" guarantee this method is built around. Same narrowing applies to coppa at line 230.
Suggest elem.canConvertToInt() / node.canConvertToInt() (already the idiom in RubiconBidder) — it returns false for both non-integral and out-of-range values, so oversized input falls through to the safe "leave untouched" path instead of emitting wrong data.
There was a problem hiding this comment.
Fixed. Went with isIntegralNumber() plus canConvertToInt(): on a double node canConvertToInt() only range-checks, so 6.5 would still truncate to 6 in asInt. The combined guard rejects fractional and out-of-range values, both covered by tests. The coppa call site is gone since coppa is no longer promoted.
| } | ||
|
|
||
| // Rebuild regs.ext keeping the typed fields and every property except the promoted ones. | ||
| private static ExtRegs removePromotedKeys(ExtRegs ext, |
There was a problem hiding this comment.
suggestion (design): when a field is already set top-level and a different value also sits in regs.ext, the ext copy is left in place — so the outbound request can carry two conflicting declarations of the same consent signal. This is deliberate and well-tested (makeHttpRequestsShouldKeepExtGppWhenTopLevelGppAlreadySetEvenIfSiblingIsPromoted), but it diverges from core's stricter rule in BidRequestOrtb25To26Converter.resolveRegsExt, which clears the ext copy of gdpr/us_privacy whenever ext had a value at all ("once superseded, always clear").
Consider matching that here: strip the ext key whenever a valid top-level value exists, not only when this call performed the promotion. Then the wire can never carry two values for one signal.
There was a problem hiding this comment.
Done. The ext key is now stripped whenever a valid top-level value exists, promoted or already set, matching core's rule. One signal never goes out with two values. Covered by tests for both gpp and gpp_sid.
| private static MultiMap makeHeaders(Device device) { | ||
| final MultiMap headers = HttpUtil.headers() | ||
| .add(HttpUtil.X_OPENRTB_VERSION_HEADER, "2.5"); | ||
| .add(HttpUtil.X_OPENRTB_VERSION_HEADER, "2.6"); |
There was a problem hiding this comment.
suggestion (non-blocking): this hard-coded "2.6" duplicates ortb-version: "2.6" in yahooAds.yaml with no link between them — which is exactly the drift that produced the bug this PR fixes (yaml said 2.6, the code did 2.5). It matches the codebase's dominant convention, so not a new problem, but at minimum consider hoisting it to a named constant next to GPP_PROPERTY (newer bidders like Msft do this). A nice follow-up would be setting this header centrally from BidderInfo.getOrtbVersion() so the ~38 hand-maintained copies across bidders collapse to one source of truth.
There was a problem hiding this comment.
Done, hoisted to an OPENRTB_VERSION constant. Agreed the BidderInfo.getOrtbVersion() route would be the real fix, but that touches every bidder, so leaving it as a follow-up.
|
|
||
| if (regs != null) { | ||
| requestBuilder.regs(modifyRegs(regs)); | ||
| requestBuilder.regs(promoteRegsExtToTopLevel(regs)); |
There was a problem hiding this comment.
nitpick (non-blocking): promoteRegsExtToTopLevel(regs) is a pure function of regs, which is captured once before the imp loop and never changes across iterations — but it's recomputed (including a full ext.getProperties() walk) once per imp. For multi-imp requests, consider hoisting it out of the loop in makeHttpRequests and passing the already-promoted Regs into modifyRequest.
There was a problem hiding this comment.
Done. Promotion is computed once in makeHttpRequests and the promoted Regs is passed into modifyRequest, with a multi-imp test pinning that every outbound request carries the same promoted values.
| "gpp_sid": [6], | ||
| "gdpr": 0 | ||
| } | ||
| "gpp": "gppstring", |
There was a problem hiding this comment.
suggestion (non-blocking): heads-up that this fixture change is bookkeeping for the removed down-conversion, not coverage of the new promotion logic. The incoming fixture test-auction-yahooads-request.json already has gpp/gpp_sid top-level, and its only ext field (gdpr) is promoted by core (BidRequestOrtb25To26Converter) before the bidder runs — so promoteRegsExtToTopLevel short-circuits on ext == null and the new gpp/gpp_sid/coppa path is never hit end-to-end.
To actually exercise it, nest them in the incoming request:
"regs": { "ext": { "gdpr": 0, "gpp": "gppstring", "gpp_sid": [6], "coppa": 0 } }and expect the flattened top-level shape here (using coppa: 0 doubles as a falsy-zero regression guard).
There was a problem hiding this comment.
Done. The incoming fixture now nests gdpr/gpp/gpp_sid in regs.ext and the expected wire asserts the flattened shape, so the test fails if the promotion breaks (checked by temporarily disabling it). coppa was left out since it is no longer promoted.
Align the regs ext-to-top promotion with core's rule for gdpr/us_privacy in BidRequestOrtb25To26Converter: an ext gpp/gpp_sid/coppa key is removed whenever the outbound request carries a top-level value for that field, whether the value was just promoted or was already set, so one signal is never sent with two conflicting values. Malformed ext values with no top-level counterpart and all unrelated regs.ext content stay untouched.
isIntegralNumber accepts long/BigInteger nodes whose asInt conversion silently overflows, so an oversized gpp_sid element or coppa value was promoted as a corrupted number. canConvertToInt rejects both non-integral and out-of-range values, letting such input fall through to the existing leave-in-ext-untouched path.
Hoist the hard-coded x-openrtb-version header value to an OPENRTB_VERSION constant, following the convention used by newer adapters, so the version lives in one visible place for the next migration.
The incoming fixture carried gpp/gpp_sid at top-level, so the bidder's ext-to-top promotion short-circuited and the integration test only covered passthrough. Nest gdpr/gpp/gpp_sid/coppa in regs.ext and expect the flattened top-level shape on the wire, so the test fails if the promotion stops working. coppa uses 0 to guard the explicit-zero case.
promoteRegsExtToTopLevel is a pure function of the request-level regs, which never changes across imp iterations, so compute it once in makeHttpRequests and pass the promoted result into modifyRequest rather than recomputing it for every impression.
coppa has been a top-level Regs field since OpenRTB 2.2 and never had a regs.ext location, so the bidder no longer promotes or strips it; an ext coppa is treated like any unknown ext property and passes through untouched. Promotion scope is now exactly gpp and gpp_sid, the two fields with a genuine 2.5 ext location.
…cases canConvertToInt alone only range-checks a double node, so an ext gpp_sid element like 6.5 passed the malformed guard and silently truncated to 6 in asInt. Combine it with isIntegralNumber so fractional values take the leave-in-ext path, and add tests for that case plus three previously unpinned behaviors: an empty top-level gpp_sid array counts as unset, per-field independence when one field is superseded while the other promotes, and the no-op pass when ext holds only unrelated properties.
coppa is no longer promoted at any type, so the wrong-type case is covered by the dedicated not-promoted test and these assertions were vacuous. The test now exercises only gpp and gpp_sid type handling.
coppa is outside this migration's scope, so drop the dedicated not-promoted test, the coppa lines in the passthrough tests, and the comment sentence about it. Its passthrough behavior is the default unknown-key path already covered by the unrelated-properties test.
Mirrors the existing gpp conflict test for the array field: when gpp_sid is set at both top-level and in regs.ext with different values, the top-level value is kept and the superseded ext copy is stripped.
An empty top-level gpp is a non-null value, so it blocks promotion and its superseded ext copy is stripped, matching core's null-check rule for us_privacy in BidRequestOrtb25To26Converter.
An empty ext gpp_sid array carries no value, so nothing is promoted and the array stays in ext untouched. Covers the node.isEmpty guard arm that no other test reached.
Promotion runs once per request and the promoted regs instance is shared by every per-imp outbound request, so pin that all of them carry the same correctly promoted values.
scr-oath
left a comment
scr-oath
left a comment
There was a problem hiding this comment.
Re-review: all requested changes addressed ✅
Thanks @MananRPatel — went through the 5 new commits and verified every point from my earlier review against the code (not just the replies):
gpp_sidint-narrowing —!isIntegralNumber() || !canConvertToInt()correctly rejects both fractional and out-of-int-range values, pinned by...ShouldNotPromoteExtValuesThatDoNotFitInIntand...WhenArrayHasFractionalElement. Good catch thatcanConvertToInt()alone would let6.5through as6— the combined guard is exactly right.- Stale ext copy / dual values —
removeSupersededKeysnow strips the ext key whenever a top-level value exists (promoted or pre-existing), matchingBidRequestOrtb25To26Converter's "once superseded, always clear." Pinned by...ShouldDropExtGppWhenTopLevelGppAlreadySet/...ShouldDropExtGppSidWhenTopLevelGppSidAlreadySet. - IT coverage — the incoming fixture now nests
gpp/gpp_sidunderregs.extand the expected wire asserts the flattened shape, so the promotion path is exercised end-to-end. - Per-imp recompute — hoisted to a single
promotedRegsinmakeHttpRequests;...ShouldApplyTheSamePromotedRegsToEveryImplocks cross-imp consistency. - Header —
OPENRTB_VERSIONconstant. Agree theBidderInfo.getOrtbVersion()route is the real fleet-wide fix; fine as a follow-up. - Dropping
coppapromotion — good call.coppahas been top-level since OpenRTB 2.0 and isn't read fromregs.extby core or the privacy services, so promoting it was defending an unreachable case. Narrower surface is the right move.
Also confirmed the YahooAdsConfiguration change (dropping usersyncerCreator / external-url) is correct — that's conformance to master, which deleted UsersyncerCreator and now auto-wires usersync from the yaml usersync: block in BidderDepsAssembler. The yaml block is intact, so cookie-sync is unaffected.
Approving. Two things still gate merge, neither about your code:
- @CTMBNara's earlier change-request — I believe the
copparemoval + the core-converter explanation resolve it, but it needs their re-review to clear. - Per prebid norms, a second committer approval.
One non-code item worth a final sign-off before this ships: confirmation from the Yahoo SSP side that top-level 2.6 privacy fields (especially the TCF consent string) are honored at their new location — the e2e 200s are necessary but not sufficient there.
4 participants
🔧 Type of changes
Migrates the Yahoo Ads bidder (and its
yssp/yahoossp/yahooAdvertisingaliases) from OpenRTB 2.5 to OpenRTB 2.6 on the wire. Three changes, in the bidder and its Spring config:x-openrtb-version: 2.5→2.6insrc/main/java/org/prebid/server/bidder/yahooads/YahooAdsBidder.java.conversionManager.convertFromAuctionSupportedVersion(bidRequest, OrtbVersion.ORTB_2_5)). That call is removed, along with the now-unusedBidRequestOrtbVersionConversionManagerdependency in the bidder and itsYahooAdsConfiguration.regs.{gpp, gpp_sid}. The oldmodifyRegs/resolveExtRegsblock re-buried these (and gdpr/us_privacy) intoregs.ext. It is replaced withpromoteRegsExtToTopLevel: a field is promoted only when it is absent at top-level and present and well-formed inregs.ext, and a key is removed fromregs.extwhenever the outbound request carries a top-level value for that field, whether the value was just promoted or was already set. This matches core's rule forgdpr/us_privacyinBidRequestOrtb25To26Converter.resolveRegsExt(top-level wins; a superseded ext copy is cleared), so one signal never goes out with two values. Typed ext-only fields (gpc,dsa), unknownregs.extproperties, and either of the two that could not be promoted and have no top-level value (e.g. a malformed, non-textualgpp, or agpp_sidarray with a non-integer element) are left inregs.extuntouched, so no value is silently dropped.No publisher-facing config change is required: bidder params (
dcn,pos) are unchanged, andbidder-config/yahooAds.yamlalready declaredortb-version: "2.6".Why
PBS-Java core up-converts every incoming request to its canonical 2.6 form (
BidRequestOrtb25To26Converter) before bidders run. The Yahoo bidder then explicitly down-converted that back to 2.5 and re-buriedgpp/gpp_sidintoregs.ext, and sentx-openrtb-version: 2.5. Net effect: even fully 2.6-aware publishers had privacy/addressability fields buried in.exton the wire.Removing the in-bidder down-conversion lets core's existing promotion reach the wire. The new
promoteRegsExtToTopLeveladditionally promotes the two regulatory fields that neither converter handles.regs.gdpr,regs.us_privacyregs.ext.*Regsuser.consent,user.eidsuser.ext.*Usersource.schainsource.ext.schainSourceimp.rwddimp.ext.prebid.is_rewarded_inventoryImpregs.gpp,regs.gpp_sidregs.ext.*RegsWhat this means for publishers
regs.gpptop-level).ext; header was2.52.6.exton the wiregdpr,us_privacy,consent,eids,schain,rwdd; the bidder additionally promotesgpp,gpp_sid; everything else passes through in.extNon-regulatory 2.6 fields the bidder does not transform (e.g.
device.sua,user.kwarray,site/app.inventorypartnerdomain,content.network,content.channel) pass through in whatever shape the publisher sent.What changed
src/main/java/org/prebid/server/bidder/yahooads/YahooAdsBidder.javasrc/main/java/org/prebid/server/spring/config/bidder/YahooAdsConfiguration.javasrc/test/java/org/prebid/server/bidder/yahooads/YahooAdsBidderTest.javasrc/test/resources/org/prebid/server/it/openrtb2/yahooads/test-auction-yahooads-request.jsonsrc/test/resources/org/prebid/server/it/openrtb2/yahooads/test-yahooads-bid-request.jsonYahooAdsBidder.javabumps the header literal, removes the down-converter call and dependency, and replaces the regs re-bury logic withpromoteRegsExtToTopLevel+gppToPromote/gppSidToPromote(each returns the value to lift, or null) andremoveSupersededKeys. The helpers operate on atoBuilder()copy ofRegs(the bidder already rebuilds the request per-impression), so they cannot affect the request other bidders see.Test plan
Automated
mvn test -Dtest=YahooAdsBidderTest38 unit tests pass. New cases cover: ext→top promotion, mixed publisher shapes (some fields top-level, some in ext), wrong-typedregs.extvalues (no promotion), a malformed value left in ext while a sibling field promotes, agpp_sidarray with a non-integer element left in ext untouched, the same field present at both top-level and ext with different values (top-level wins, superseded ext copy stripped), a fractional or out-of-int-rangegpp_sidelement treated as malformed, an empty extgpp_sidarray not promotable, an empty top-levelgppcounting as set, the same promoted regs applied to every imp of a multi-imp request, and the no-extshort-circuit.mvn test -Dtest=YahooAdsTestintegration test passes against the updated 2.6 wire fixture.End-to-end against the Yahoo Ads SSP
A build of this branch was run as a local PBS-Java process (not Docker) and pointed at the Yahoo Ads SSP endpoint. For each scenario a request was POSTed to
/openrtb2/auctionwithext.prebid.debug: 1, and the exact bytes the bidder put on the wire were read back fromext.debug.httpcalls.yahooAds[*](request body + headers). Every outbound carriedx-openrtb-version: 2.6and the SSP returned HTTP 200. Bid presence varies with demand and is not asserted here.regs.gpp/regs.gpp_sidpromoted to top-level and removed fromregs.ext; core-promoted fields (gdpr,us_privacy) at top-level; unrelatedregs.extkeys (gpc,dsa) left inregs.extplcmt=2andplcmt=4podid,podseq,slotinpod,maxseq,poddur,mincpmpersec,rqddurs) intactapp.idoverride applied (same mechanism assite.id); 2.6 wireHow to verify locally
Test fixture changes
test-auction-yahooads-request.json(integration, incoming):gdpr/gpp/gpp_sidnow arrive nested inregs.ext, so the test exercises the promotion end-to-end instead of plain passthrough.test-yahooads-bid-request.json(integration, expected wire): outbound now carriesregs.gpp/regs.gpp_sid/regs.gdprat top-level (previously underregs.ext).YahooAdsBidderTest.java: header assertion2.5→2.6; the "remove/overwrite regs ext" tests are replaced with promotion + passthrough tests described above.Backward compatibility
No breaking changes.
dcn,pos) unchanged.makeBids) unchanged.gpp/gpp_sidat top-level; all other fields pass through unchanged in.ext.Aliases
yssp,yahoosspandyahooAdvertisingare declared as aliases ofyahooAdsinbidder-config/yahooAds.yamland share the sameYahooAdsBidderinstance, so all alias codes send the 2.6 wire after this PR.For contact: prebid-tech-team@yahooinc.com
Implemented with assistance from Claude Code.